-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Lint against &T
to &mut T
and &T
to &UnsafeCell<T>
transmutes (rebase)
#143343
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Lint against &T
to &mut T
and &T
to &UnsafeCell<T>
transmutes (rebase)
#143343
Conversation
r? @SparrowLii rustbot has assigned @SparrowLii. Use |
The Miri subtree was changed cc @rust-lang/miri |
This comment has been minimized.
This comment has been minimized.
553a558
to
abcc149
Compare
This comment has been minimized.
This comment has been minimized.
abcc149
to
0b550dd
Compare
This adds the lint against `&T`->`&UnsafeCell<T>` transmutes, and also check in struct fields, and reference casts (`&*(&a as *const u8 as *const UnsafeCell<u8>)`). The code is quite complex; I've tried my best to simplify and comment it. This is missing one parts: array transmutes. When transmuting an array, this only consider the first element. The reason for that is that the code is already quite complex, and I didn't want to complicate it more. This catches the most common pattern of transmuting an array into an array of the same length with type of the same size; more complex cases are likely not properly handled. We could take a bigger sample, for example the first and last elements to increase the chance that the lint will catch mistakes, but then the runtime complexity becomes exponential with the nesting of the arrays (`[[[[[T; 2]; 2]; 2]; 2]; 2]` has complexity of O(2**5), for instance).
0b550dd
to
3495b59
Compare
I'm not familiar with lints so I'd like to roll this |
See also #133653, which appears to be a more restricted subset of this lint, I have not had the bandwidth to look into this. |
@@ -121,6 +121,7 @@ lint_builtin_missing_doc = missing documentation for {$article} {$desc} | |||
lint_builtin_mutable_transmutes = | |||
transmuting &T to &mut T is undefined behavior, even if the reference is unused, consider instead using an UnsafeCell |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
transmuting &T to &mut T is undefined behavior, even if the reference is unused, consider instead using an UnsafeCell | |
transmuting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an UnsafeCell |
@@ -934,6 +935,11 @@ lint_unsafe_attr_outside_unsafe = unsafe attribute used without unsafe | |||
.label = usage of unsafe attribute | |||
lint_unsafe_attr_outside_unsafe_suggestion = wrap the attribute in `unsafe(...)` | |||
lint_unsafe_cell_transmutes = | |||
converting &T to &UnsafeCell<T> is error-prone, rarely intentional and may cause undefined behavior |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
converting &T to &UnsafeCell<T> is error-prone, rarely intentional and may cause undefined behavior | |
converting `&T` to `&UnsafeCell<T>` is error-prone, rarely intentional and may cause undefined behavior |
LL | let _: Foo<&'static UnsafeCell<u8>> = unsafe { transmute(Other(&1u8, &1u8)) }; | ||
| ^^^^^^^^^ | ||
| | ||
= note: conversion from `*main::Other.1` to `*main::Foo<&UnsafeCell<u8>>.b.0.0` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit hard to untangle the formatting machinery and I feel like this could be clearer. Here's a diagnostic I'd like to see instead:
= note: conversion from `*main::Other.1` to `*main::Foo<&UnsafeCell<u8>>.b.0.0` | |
note: this field... (`1` in `main::Other`) | |
--> file.rs:LL:CC | |
| | |
LL | struct Other(&'static u8, &'static u8); | |
| ^^^^^^^^^^^ | |
| | |
= note: this field has type `&u8` | |
| | |
note: ...is being converted to this (`b.0.0` in `main::Foo<&UnsafeCell<u8>>`) | |
--> file.rs:LL:CC | |
| | |
LL | struct Baz<T>(T); | |
| ^ | |
| | |
= note: this field has type `&UnsafeCell<u8>` |
Storing the relevant information for this (instead of what is done currently) would probably reduce the complexity in the code.
This is a rebase of #128351 with the new lint's name being reverted back to
unsafe_cell_transmutes
, as requested by T-lang.Original description: